Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add textobject for entries/elements of list-like things #8150

Conversation

EpocSquadron
Copy link
Contributor

@EpocSquadron EpocSquadron commented Sep 3, 2023

Closes #3006

Explanation

This introduces a new tree-sitter textobject query for elements or "entries" of list-like (data structure like) objects. This might be array elements, tuple entries, key-value pairs within object literals, enum members, etc. The "inside" subtype can mean the individual parts of key-value pairs where that is applicable, or left undefined for things like Rust arrays where they are not. It is also implemented for movements (] and [).

I've implemented the queries for a number of core languages, but note that deciding on whether something gets to be considered a data structure can be subjective. In particular, some languages encourage data structures implemented as classes where the properties are the entries/elements. However, because it's unclear how methods should interact with this feature, I chose the conservative path and did not implement the queries for classes in the languages that have such a concept. I suspect that native users of each language will want to evolve what is considered a data structure like entry over time.

This feature also introduces textobjects for a number of languages that did not have textobjects defined at all before, such as JSON and TOML, where no other query type makes sense.

@EpocSquadron EpocSquadron force-pushed the epocsquadron/textobject-pairings branch from ec1e4f4 to 82728a0 Compare March 13, 2024 00:13
@EpocSquadron EpocSquadron changed the title Add textobject for elements of list-like things Add textobject for entries/elements of list-like things Mar 13, 2024
@EpocSquadron EpocSquadron marked this pull request as ready for review March 13, 2024 00:22
@EpocSquadron
Copy link
Contributor Author

I had to let this sit a bit longer to gain clarity, and also life got in the way for quite a while. However, I think I've narrowed this down to a sensible implementation and I think its ready for more eyes.

pascalkuthe
pascalkuthe previously approved these changes Mar 13, 2024
Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks fine to me since it's fairly simple.

I think conceptually it seems useful to have a textonject like this.

Will try to test this a bit if the queries make sense to me.

Can you clarify what you mean with that comment avkht class fields? I see rust field declarations are covered by these textobjects which sound just like class fields to me.

@EpocSquadron
Copy link
Contributor Author

@pascalkuthe certainly! Perhaps the best way to explain is to look at some specific examples that don't fit the mold precisely.

In JavaScript, there are a number of ways to create objects. One of these is the "object literal" syntax, which looks a lot like a hash table definition in other languages:

const person = {
  name: 'Ragesh',
  age: '52',
};

In this instance, it is easy to say that the person object represents a data structure, where the key-value pairs form our textobject's subject. However, consider also that JavaScript has first-class functions, allowing us to construct an object with a more traditional class-oriented shape:

const person = {
  name: 'Yvette',
  age: '33',
  speak: () => console.log('Bonjour!')
}

Here the speak member is an invokable function, veritably a method. I chose not to try to distinguish this type of member as it makes the queries much more difficult to write (and probably less performant). So methods on an object in this form get to be recognized as "entries in a data structure-like thing".

Now consider that in recent years JavaScript grew some class-based syntactic sugar to make constructing objects more familiar to those coming from class-based OO languages. We now can express the same object this way:

class Person {
  constructor(name, age) {
    this.name = name
    this.age = age
  }

  speak() {
    console.log('你好')
  }
}

const person = new Person('Jian', 19)

Does it now make sense to be able to jump to the assigments for this.name and this.age? Should the speak method be considered an entry/element here too? Or private members like this?

class Fish {
  #numFins = 3
  getNumFins() {
    return this.#numFins
  }
}

Initially, my answer was "yes, these should count", but as I tried it in more contexts it just felt kind of inconsistent and awkward to use. Even where you might expect it to act like entries in a data structure there can be awkwardness, for example with a PHP Data Transfer Object pattern:

readonly class BlogData
{
    public function __construct(
        public string $title,
        public Status $status,
        public ?DateTimeImmutable $publishedAt = null,
    ) {}
}

Here you can see that due to PHP's constructor property promotion (adding visibility to the constructor signature means those arguments immediately become properties on that class with said visibility), we now have an overlap between the definition of the "argument/parameter" textobject we already have and this new "entry/element" textobject.

In the specific case of Rust, field definitions on a struct feel to me like they belong, as structs are often thought of as data structures more consistently, and the fact that method impls are separated into their own impl block separated from the definition of the fields makes the kind of awkwardness I show above less bothersome in practice. I am open to backing off of that as well though.

In other words, getting a useful definition of what does and does not belong is difficult to achieve. I gave it my best shot for the covered languages, given that my experience with many of them is surface level.

@pascalkuthe
Copy link
Member

That makes sense to me. I guess you could in theory view every assignment as a key value pair which I definitely wouldn't want the captures to do.

Rust is fairly rigorous and I think I agree with the way its defined there and python makes sense too. I don't have enough experience with js/php to really comment on that but your explanation sounds reasonable.

@pascalkuthe pascalkuthe added this to the next milestone Mar 17, 2024
@the-mikedavis the-mikedavis merged commit 1abb64e into helix-editor:master Mar 31, 2024
6 checks passed
@EpocSquadron EpocSquadron deleted the epocsquadron/textobject-pairings branch March 31, 2024 16:55
Desdaemon pushed a commit to Desdaemon/helix that referenced this pull request Apr 3, 2024
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 4, 2024
postsolar pushed a commit to postsolar/helix that referenced this pull request Apr 20, 2024
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
Vulpesx pushed a commit to Vulpesx/helix that referenced this pull request Jun 7, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tree-sitter based textobject for pairings and fields
3 participants